Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: initial implementation for WebM player for recordings #832

Merged

Conversation

nicolesabourin1
Copy link
Contributor

@nicolesabourin1 nicolesabourin1 commented Apr 30, 2024

Adds a video and xterm player at the GET /jet/jrec/play endpoint which supports multiple videos and builds the page dynamically based on the type of recording.

Issue: DGW-110

<head>
<meta name="viewport" content="width=device-width">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/xterm-player@latest/dist/css/xterm-player.min.css" />
<script src="https://cdn.jsdelivr.net/npm/xterm@4.4.0/lib/xterm.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolesabourin1 I'm not sure how we currently handle the dependencies for the rest of our web frontend, but we'd need to build and include external dependencies alongside the rest of the frontend such that it can work without Internet access. No external CDN delivery can be used for this, as this causes problems for GDPR compliance in addition to breaking things when used without Internet access

Copy link
Member

@CBenoit CBenoit May 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that we should use the @xterm/xterm npm package: https://www.npmjs.com/package/@xterm/xterm

It’s then possible to use ES6 module syntax:

import { Terminal } from '@xterm/xterm';

Can you confirm? @kristahouse

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you can use ES6 module syntax to import modules in a plain JavaScript setup within an HTML page.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how quickly can this be fixed? we're looking into making a new release soon, and it would be great to have a V1 of the web player included in it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is we need a way to host the static web files inside the gateway web server for the xterm js and css files, just like we did for the webapp static files, but in this case the webapp may not be enabled, @CBenoit will need to make an endpoint for the static files.

Copy link
Member

@CBenoit CBenoit May 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pauldumais Just like for the webapp, the GET /jet/jrec/play/* endpoint is serving all the files found in the player/ folder. Is it not enough in this case? It’s working really the same as the GET /jet/webapp/client/* endpoint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CBenoit awesome, I just added the xterm player files locally to the folder

req.send(null);
}

function convertTRPtoCast(fileArray) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have our answer, Paul baked in a live TRP to .cast converter directly in the Javascript of the player, so it'll be able to play the older TRP file format as well :)

@CBenoit CBenoit changed the title [DGW-110] Recording-Player-Initial-webm-player-for-RDP feat: initial implementation for WebM player for recordings May 2, 2024
@CBenoit CBenoit requested a review from kristahouse May 2, 2024 05:27
@CBenoit CBenoit enabled auto-merge (squash) May 2, 2024 05:28
@CBenoit CBenoit merged commit 58362b9 into master May 6, 2024
20 checks passed
@CBenoit CBenoit deleted the DGW-110-Recording-Player-Initial-webm-player-for-RDP branch May 6, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants